Automated Test: oauth-state-secure #312
Conversation
…#67876) We're adding one more step in the GitHub integration installation pipeline, namely GitHub OAuth2 authorize. This is transparent from the UX perspective as the data exchange happens without user interaction. The pipeline will now fail in these cases: - If there is a mismatch between currently authenticated GitHub user (derived from OAuth2 authorize step) and the user who installed the GitHub app (https://github.com/apps/sentry-io) - If there is a mismatch between `state` parameter supplied by user and pipeline signature - If GitHub could not generate correct `access_token` from the `code` (wrong or attempt of re-use of `code`). In all those cases, this error is shown: 
📝 WalkthroughWalkthroughGitHub integration now supports OAuth-based login flow during installation setup. New error handling infrastructure and helper functions provide standardized failure page rendering. The pipeline is updated to include an OAuthLoginView alongside existing installation views. A simplification in pipeline advancer removes a list constant for direct GitHub provider comparison. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Sentry as Sentry<br/>(OAuthLoginView)
participant GitHub as GitHub<br/>OAuth
participant Pipeline as Pipeline<br/>State
Client->>Sentry: Initiate GitHub setup
Sentry->>GitHub: Redirect to authorize<br/>(client_id, state, scope)
GitHub->>Client: Authorization prompt
Client->>GitHub: User grants permission
GitHub->>Sentry: Callback with code & state
Sentry->>Sentry: Validate state parameter
Sentry->>GitHub: Exchange code for token
GitHub->>Sentry: Access token response
Sentry->>GitHub: Fetch user info
GitHub->>Sentry: User data (login, etc.)
Sentry->>Pipeline: Persist authenticated<br/>user & token
Sentry->>Client: Proceed to next<br/>pipeline view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/sentry/integrations/github/test_integration.py (1)
378-396:⚠️ Potential issue | 🟡 MinorTest name
test_installation_not_foundis now misleading — it tests OAuth state mismatch instead.The 404 stub for the installation endpoint (Line 381–383) is never reached because the OAuth state
"ddd023d87a913d5226e2a882c4c4cc05"doesn't match the pipeline signature, causingOAuthLoginViewto reject the request before the installation lookup. The test now validates state mismatch, not a 404 on the installation. Consider renaming it (e.g.,test_invalid_oauth_state) or adding a separate test that exercises the actual installation-not-found path through a valid OAuth flow.
🤖 Fix all issues with AI agents
In `@src/sentry/integrations/github/integration.py`:
- Around line 493-505: The code assumes integration.metadata["sender"] always
exists and will raise KeyError for older integrations; update the check that
compares pipeline.fetch_state("github_authenticated_user") with
integration.metadata["sender"]["login"] to safely handle missing metadata by
using dict.get (e.g. integration.metadata.get("sender", {}) or retrieving sender
= integration.metadata.get("sender")) or by catching KeyError, and if sender or
sender["login"] is absent treat it as a mismatch and return error(request,
self.active_organization); ensure you reference Integration and
pipeline.fetch_state("github_authenticated_user") when making the change so the
comparison remains robust.
- Around line 447-454: The dispatch method now only checks "if installation_id
is None" which lets empty strings through and binds them via
pipeline.bind_state, causing downstream get_installation_info calls to hit an
invalid /app/installations/ endpoint; change the check to reject empty strings
(e.g., use a truthy check like "if not installation_id" or explicitly validate
installation_id.strip() is non-empty) before redirecting, ensuring
pipeline.fetch_state/pipeline.bind_state only see a valid non-empty
installation_id used by get_installation_info.
In `@tests/sentry/integrations/github/test_integration.py`:
- Around line 398-454: The test test_github_user_mismatch fails because the
hardcoded HTTP_X_HUB_SIGNATURE value does not match the HMAC-SHA1 of the webhook
payload (INSTALLATION_EVENT_EXAMPLE) given the default empty webhook secret,
causing the webhook to be rejected before the user-mismatch logic runs; to fix,
either set the webhook secret explicitly via
options.set("github-app.webhook-secret", "<secret>") in the test setup and use
the matching signature header, or compute the correct HMAC-SHA1 for
INSTALLATION_EVENT_EXAMPLE with the secret used and replace the
HTTP_X_HUB_SIGNATURE header in the client.post call so the signature validation
in the GitHub webhook endpoint passes and the test exercises
GitHubIntegrationTest::test_github_user_mismatch logic.
🧹 Nitpick comments (2)
src/sentry/integrations/github/integration.py (2)
401-409: OAuth authorize URL is constructed via string concatenation without URL-encoding.While the values (
github_client_id,state,redirect_uri) are likely safe in practice (alphanumeric or clean URL paths), query parameter values should be properly URL-encoded to prevent breakage with unexpected characters.♻️ Suggested fix using `urlencode`
+from urllib.parse import urlencode ... if not request.GET.get("state"): state = pipeline.signature redirect_uri = absolute_uri( reverse("sentry-extension-setup", kwargs={"provider_id": "github"}) ) + params = urlencode({ + "client_id": github_client_id, + "state": state, + "redirect_uri": redirect_uri, + }) return self.redirect( - f"{ghip.get_oauth_authorize_url()}?client_id={github_client_id}&state={state}&redirect_uri={redirect_uri}" + f"{ghip.get_oauth_authorize_url()}?{params}" )Note:
urlencodeis already imported at line 7 asparse_qsl— just addurlencodeto that import.
425-429: Bareexcept Exceptionswallows all errors during token parsing.Per the Ruff hint (BLE001), catching the broad
Exceptionhere could mask unexpected errors (e.g., encoding issues, attribute errors). The likely failure modes are network/parsing errors, so a narrower catch would be safer.♻️ Suggested narrower exception handling
try: body = safe_urlread(req).decode("utf-8") payload = dict(parse_qsl(body)) - except Exception: + except (ValueError, AttributeError, UnicodeDecodeError): payload = {}
| def dispatch(self, request: Request, pipeline: Pipeline) -> HttpResponse: | ||
| if "installation_id" not in request.GET: | ||
| installation_id = request.GET.get( | ||
| "installation_id", pipeline.fetch_state("installation_id") | ||
| ) | ||
| if installation_id is None: | ||
| return self.redirect(self.get_app_url()) | ||
|
|
||
| pipeline.bind_state("installation_id", installation_id) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how installation_id is used downstream after being bound to state
rg -n "installation_id" src/sentry/integrations/github/integration.py -C2Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 1895
🏁 Script executed:
#!/bin/bash
# Look at build_integration method to see if there's validation handling
sed -n '351,380p' src/sentry/integrations/github/integration.pyRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 1371
Changed from if not installation_id to if installation_id is None — this allows empty strings to proceed.
request.GET.get("installation_id") returns None when absent but "" when present but empty. The old check (if not installation_id) rejected both None and "", while the new check only rejects None. An empty string would now:
- Bypass the redirect at line 451-452
- Bind an empty string to pipeline state (line 454)
- Reach
get_installation_info()with an empty string, creating an invalid API call to/app/installations/(line 348)
This is inconsistent with the pattern used earlier at line 398 (if installation_id:) and creates a behavioral regression. Either revert to the truthy check or explicitly validate that installation_id is non-empty.
🤖 Prompt for AI Agents
In `@src/sentry/integrations/github/integration.py` around lines 447 - 454, The
dispatch method now only checks "if installation_id is None" which lets empty
strings through and binds them via pipeline.bind_state, causing downstream
get_installation_info calls to hit an invalid /app/installations/ endpoint;
change the check to reject empty strings (e.g., use a truthy check like "if not
installation_id" or explicitly validate installation_id.strip() is non-empty)
before redirecting, ensuring pipeline.fetch_state/pipeline.bind_state only see a
valid non-empty installation_id used by get_installation_info.
| try: | ||
| integration = Integration.objects.get( | ||
| external_id=installation_id, status=ObjectStatus.ACTIVE | ||
| ) | ||
| except Integration.DoesNotExist: | ||
| return error(request, self.active_organization) | ||
|
|
||
| # Check that the authenticated GitHub user is the same as who installed the app. | ||
| if ( | ||
| pipeline.fetch_state("github_authenticated_user") | ||
| != integration.metadata["sender"]["login"] | ||
| ): | ||
| return error(request, self.active_organization) |
There was a problem hiding this comment.
integration.metadata["sender"] will KeyError if the integration was created before this change or via a path that doesn't set "sender".
build_integration only sets metadata["sender"] conditionally (Line 376–377: if state.get("sender")). Pre-existing integrations or those created through flows that don't populate sender in state will lack this key, causing a KeyError at Line 503.
🐛 Proposed fix — guard against missing "sender" metadata
# Check that the authenticated GitHub user is the same as who installed the app.
- if (
- pipeline.fetch_state("github_authenticated_user")
- != integration.metadata["sender"]["login"]
- ):
+ sender = integration.metadata.get("sender")
+ if not sender or (
+ pipeline.fetch_state("github_authenticated_user")
+ != sender.get("login")
+ ):
return error(request, self.active_organization)🤖 Prompt for AI Agents
In `@src/sentry/integrations/github/integration.py` around lines 493 - 505, The
code assumes integration.metadata["sender"] always exists and will raise
KeyError for older integrations; update the check that compares
pipeline.fetch_state("github_authenticated_user") with
integration.metadata["sender"]["login"] to safely handle missing metadata by
using dict.get (e.g. integration.metadata.get("sender", {}) or retrieving sender
= integration.metadata.get("sender")) or by catching KeyError, and if sender or
sender["login"] is absent treat it as a mismatch and return error(request,
self.active_organization); ensure you reference Integration and
pipeline.fetch_state("github_authenticated_user") when making the change so the
comparison remains robust.
| @responses.activate | ||
| def test_github_user_mismatch(self): | ||
| self._stub_github() | ||
|
|
||
| # Emulate GitHub installation | ||
| init_path_1 = "{}?{}".format( | ||
| reverse( | ||
| "sentry-organization-integrations-setup", | ||
| kwargs={ | ||
| "organization_slug": self.organization.slug, | ||
| "provider_id": self.provider.key, | ||
| }, | ||
| ), | ||
| urlencode({"installation_id": self.installation_id}), | ||
| ) | ||
| self.client.get(init_path_1) | ||
|
|
||
| webhook_event = json.loads(INSTALLATION_EVENT_EXAMPLE) | ||
| webhook_event["installation"]["id"] = self.installation_id | ||
| webhook_event["sender"]["login"] = "attacker" | ||
| resp = self.client.post( | ||
| path="/extensions/github/webhook/", | ||
| data=json.dumps(webhook_event), | ||
| content_type="application/json", | ||
| HTTP_X_GITHUB_EVENT="installation", | ||
| HTTP_X_HUB_SIGNATURE="sha1=d184e6717f8bfbcc291ebc8c0756ee446c6c9486", | ||
| HTTP_X_GITHUB_DELIVERY="00000000-0000-4000-8000-1234567890ab", | ||
| ) | ||
| assert resp.status_code == 204 | ||
|
|
||
| # Validate the installation user | ||
| user_2 = self.create_user("foo@example.com") | ||
| org_2 = self.create_organization(name="Rowdy Tiger", owner=user_2) | ||
| self.login_as(user_2) | ||
| init_path_2 = "{}?{}".format( | ||
| reverse( | ||
| "sentry-organization-integrations-setup", | ||
| kwargs={ | ||
| "organization_slug": org_2.slug, | ||
| "provider_id": self.provider.key, | ||
| }, | ||
| ), | ||
| urlencode({"installation_id": self.installation_id}), | ||
| ) | ||
| setup_path_2 = "{}?{}".format( | ||
| self.setup_path, | ||
| urlencode( | ||
| {"code": "12345678901234567890", "state": "9cae5e88803f35ed7970fc131e6e65d3"} | ||
| ), | ||
| ) | ||
| with self.feature({"organizations:customer-domains": [org_2.slug]}): | ||
| resp = self.client.get(init_path_2) | ||
| resp = self.client.get(setup_path_2) | ||
| self.assertTemplateUsed(resp, "sentry/integrations/github-integration-failed.html") | ||
| assert resp.status_code == 200 | ||
| assert b'window.opener.postMessage({"success":false' in resp.content | ||
| assert b"Invalid installation request." in resp.content |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the webhook secret configuration used in tests
rg -n "webhook.secret\|GITHUB_WEBHOOK_SECRET\|github-app.webhook-secret" --type=py -C2 -g '!node_modules/**'Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Look for the test class setup and webhook configuration
cd tests/sentry/integrations/github
rg -n "_stub_github\|INSTALLATION_EVENT_EXAMPLE\|webhook" test_integration.py -B2 -A5 | head -100Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Find where the test fixture sets the webhook secret
rg -n "webhook.*secret\|secret.*webhook" tests/sentry/integrations/github/test_integration.py -i -B3 -A3Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Get the test file to understand the structure
wc -l tests/sentry/integrations/github/test_integration.pyRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Read the test class and its setup methods
head -100 tests/sentry/integrations/github/test_integration.pyRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 3662
🏁 Script executed:
#!/bin/bash
# Search for where INSTALLATION_EVENT_EXAMPLE is defined
rg "INSTALLATION_EVENT_EXAMPLE" tests/sentry/integrations/github/test_integration.py -B3 -A3Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 592
🏁 Script executed:
#!/bin/bash
# Look for class definition and any setup/fixture methods
rg "^class |def setUp|def setup|@pytest.fixture" tests/sentry/integrations/github/test_integration.py -n | head -20Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 244
🏁 Script executed:
#!/bin/bash
# Find the fixtures.github file
find . -name "fixtures.py" -o -name "github.py" | grep fixturesRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Search for _stub_github method implementation
rg "_stub_github" tests/sentry/integrations/github/test_integration.py -A 30Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 7260
🏁 Script executed:
#!/bin/bash
# Look for webhook secret in the entire test directory
rg "webhook.*secret\|HUB_SIGNATURE\|d184e6717f8bfbcc291ebc8c0756ee446c6c9486" --type=py tests/sentry/integrations/github/ -B2 -A2Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Read the fixtures/github.py file to understand INSTALLATION_EVENT_EXAMPLE
cat fixtures/github.py | head -100Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 3103
🏁 Script executed:
#!/bin/bash
# Find the webhook endpoint handler
rg "extensions/github/webhook" --type=py -B5 -A10Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 50397
🏁 Script executed:
#!/bin/bash
# Look for webhook secret or signature validation in the GitHub integration
rg "X_HUB_SIGNATURE\|webhook.*secret\|verify.*signature" src/sentry/integrations/github/ --type=py -B2 -A5Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Check the entire GitHubIntegrationTest class for webhook secret setup
sed -n '85,1112p' tests/sentry/integrations/github/test_integration.py | grep -n "webhook.*secret\|github-app\|options.set"Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Find and read INSTALLATION_EVENT_EXAMPLE from fixtures
rg "INSTALLATION_EVENT_EXAMPLE.*=" fixtures/github.py -A 80 | head -100Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 3902
🏁 Script executed:
#!/bin/bash
# Check if there's a default webhook secret value
rg "github-app.webhook-secret" src/ --type=py -B2 -A2Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 979
🏁 Script executed:
#!/bin/bash
# Calculate the HMAC signature for the webhook event to verify correctness
python3 << 'EOF'
import json
import hmac
import hashlib
# The INSTALLATION_EVENT_EXAMPLE from fixtures
event_json = """{
"action": "created",
"installation": {
"id": 2,
"account": {
"login": "octocat",
"id": 1,
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
},
"access_tokens_url": "https://api.github.com/installations/2/access_tokens",
"repositories_url": "https://api.github.com/installation/repositories"
},
"sender": {
"login": "attacker",
"id": 1,
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
}
}"""
# Test with default empty secret
secret = ""
payload = event_json.encode('utf-8')
signature = hmac.new(secret.encode('utf-8'), payload, hashlib.sha1).hexdigest()
print(f"Calculated signature (empty secret): sha1={signature}")
print(f"Hardcoded signature in test: sha1=d184e6717f8bfbcc291ebc8c0756ee446c6c9486")
print(f"Match: {signature == 'd184e6717f8bfbcc291ebc8c0756ee446c6c9486'}")
# Also test with the secret from test_webhooks
secret2 = "b3002c3e321d4b7880360d397db2ccfd"
signature2 = hmac.new(secret2.encode('utf-8'), payload, hashlib.sha1).hexdigest()
print(f"\nCalculated signature (b3002c3e secret): sha1={signature2}")
EOFRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 349
🏁 Script executed:
#!/bin/bash
# Check if GitHubIntegrationTest class has any setUp or fixture that sets webhook secret
head -300 tests/sentry/integrations/github/test_integration.py | tail -150Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 6169
🏁 Script executed:
#!/bin/bash
# Check how the webhook endpoint validates signatures
rg "X_HUB_SIGNATURE\|verify.*signature" src/sentry/integrations/github/webhook.py -B5 -A10Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Look at the dispatch/post method in the webhook handler
sed -n '1,100p' src/sentry/integrations/github/webhook.pyRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 3963
🏁 Script executed:
#!/bin/bash
# Continue reading the webhook file to see signature verification
sed -n '100,300p' src/sentry/integrations/github/webhook.pyRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 7692
🏁 Script executed:
#!/bin/bash
# Find the Endpoint class that handles the actual webhook POST
rg "class.*Endpoint.*webhook\|def post.*Request" src/sentry/integrations/github/webhook.py -A20 | head -80Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/antml:thinking>
Let me search for the GitHubInstallation class and dispatch method mentioned in the review
</thinking>
#!/bin/bash
# Search for GitHubInstallation.dispatch mentioned in the review
rg "class GitHubInstallation\|def dispatch" src/sentry/integrations/github/ --type=py -B2 -A15Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Search for GitHubInstallation.dispatch and the webhook endpoint
rg "class GitHubInstallation\|def dispatch" src/sentry/integrations/github/ --type=py -B2 -A15Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Search for the Endpoint that handles the webhook POST
rg "@all_silo_endpoint\|class.*Endpoint" src/sentry/integrations/github/webhook.py -A20Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Look at the end of the webhook.py file to find the Endpoint
tail -200 src/sentry/integrations/github/webhook.pyRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 7801
🏁 Script executed:
#!/bin/bash
# Search for class names in webhook.py
rg "^class " src/sentry/integrations/github/webhook.py -nRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 274
🏁 Script executed:
#!/bin/bash
# Check if GitHubIntegrationTest or its parent class mocks or disables signature validation
rg "is_valid_signature\|get_secret\|mock.*signature" tests/sentry/integrations/github/test_integration.py -B5 -A5Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Check the parent class IntegrationTestCase to see if it has special webhook handling
rg "class IntegrationTestCase" src/sentry/testutils/cases.py -A50Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 1839
🏁 Script executed:
#!/bin/bash
# Calculate what secret would produce the hardcoded signature
python3 << 'EOF'
import json
import hmac
import hashlib
# The INSTALLATION_EVENT_EXAMPLE modified with sender.login = "attacker"
event_json = """{
"action": "created",
"installation": {
"id": 2,
"account": {
"login": "octocat",
"id": 1,
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
},
"access_tokens_url": "https://api.github.com/installations/2/access_tokens",
"repositories_url": "https://api.github.com/installation/repositories"
},
"sender": {
"login": "attacker",
"id": 1,
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
}
}"""
# The hardcoded signature
target_signature = "d184e6717f8bfbcc291ebc8c0756ee446c6c9486"
# Try common test secrets
test_secrets = [
"",
"test",
"secret",
"github-webhook-secret",
"b3002c3e321d4b7880360d397db2ccfd",
]
payload = event_json.encode('utf-8')
for secret in test_secrets:
sig = hmac.new(secret.encode('utf-8'), payload, hashlib.sha1).hexdigest()
match = "✓" if sig == target_signature else " "
print(f"[{match}] secret='{secret}': {sig}")
EOFRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 427
🏁 Script executed:
#!/bin/bash
# Check if the test mocks the webhook endpoint or signature validation
rg "mock\|patch\|@responses" tests/sentry/integrations/github/test_integration.py | grep -A3 -B3 "test_github_user_mismatch"Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Look at the actual test method to see the full context
sed -n '398,454p' tests/sentry/integrations/github/test_integration.pyRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 2464
🏁 Script executed:
#!/bin/bash
# Check if there are any test settings or middleware that disable webhook signature validation
rg "github.*webhook.*secret\|GITHUB.*SECRET" src/sentry/settings/ tests/settings.py -B2 -A2Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 203
🏁 Script executed:
#!/antml:thinking>
Let me check if the test actually passes by looking for any test configuration that sets the webhook secret.
</thinking>
#!/bin/bash
# Check if the test uses pytest fixtures or settings that configure the webhook secret
grep -r "github-app.webhook-secret" tests/sentry/integrations/github/test_integration.pyRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Check what happens when an empty secret is used - does signature validation pass or fail?
# Let's look at is_valid_signature more carefully
python3 << 'EOF'
import hmac
import hashlib
from django.utils.crypto import constant_time_compare
# Simulate what the endpoint does
event_json = """{
"action": "created",
"installation": {
"id": 2,
"account": {
"login": "octocat",
"id": 1,
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
},
"access_tokens_url": "https://api.github.com/installations/2/access_tokens",
"repositories_url": "https://api.github.com/installation/repositories"
},
"sender": {
"login": "attacker",
"id": 1,
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
}
}"""
body = event_json.encode('utf-8')
secret = ""
signature = "d184e6717f8bfbcc291ebc8c0756ee446c6c9486"
# This is what is_valid_signature does
expected = hmac.new(key=secret.encode("utf-8"), msg=body, digestmod=hashlib.sha1).hexdigest()
print(f"Expected signature: {expected}")
print(f"Provided signature: {signature}")
print(f"Match: {expected == signature}")
# The constant_time_compare would return False
print(f"Validation would return: {expected == signature}")
EOFRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 203
🏁 Script executed:
#!/bin/bash
# Check the test configuration
python3 << 'EOF'
import hmac
import hashlib
event_json = """{
"action": "created",
"installation": {
"id": 2,
"account": {
"login": "octocat",
"id": 1,
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
},
"access_tokens_url": "https://api.github.com/installations/2/access_tokens",
"repositories_url": "https://api.github.com/installation/repositories"
},
"sender": {
"login": "attacker",
"id": 1,
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
}
}"""
body = event_json.encode('utf-8')
secret = ""
signature = "d184e6717f8bfbcc291ebc8c0756ee446c6c9486"
expected = hmac.new(key=secret.encode("utf-8"), msg=body, digestmod=hashlib.sha1).hexdigest()
print(f"Expected signature (empty secret): {expected}")
print(f"Provided signature in test: {signature}")
print(f"Match: {expected == signature}")
EOFRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 249
🏁 Script executed:
#!/bin/bash
# Check settings files to see if there's a default secret for tests
find . -name "settings*.py" -o -name "conftest.py" | head -20Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 340
🏁 Script executed:
#!/bin/bash
# Look for the test runner configuration
rg "github-app" . --type=py | grep -i "option\|setting\|default" | head -20Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 2199
🏁 Script executed:
#!/bin/bash
# Check if GitHubIntegrationTest is using responses library to mock the webhook endpoint
rg "@responses.activate" tests/sentry/integrations/github/test_integration.py -B5 | grep -A5 "test_github_user_mismatch"Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/an/bash
# Check if the webhook endpoint is being mocked/patched in the test
rg "mock\|patch\|responses.add" tests/sentry/integrations/github/test_integration.py | grep -i "webhook\|endpoint\|signature" -B2 -A2Repository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Look for how responses library is configured in tests/conftest.py
head -100 tests/conftest.pyRepository: admin-coderabbit/coderabbit-sentry-mirror
Length of output: 3634
The hardcoded webhook HMAC signature is incorrect and will cause the test to fail.
The test expects resp.status_code == 204, but the signature sha1=d184e6717f8bfbcc291ebc8c0756ee446c6c9486 does not match the HMAC-SHA1 of the webhook payload. Since GitHubIntegrationTest does not call options.set("github-app.webhook-secret", ...) (unlike other webhook tests), it uses the default empty secret. With an empty secret, the correct signature would be sha1=5edb0b1e7ce2ee0fabc7805215f2a1a87c73c2fd. The endpoint's signature validation will reject the request with HTTP 401 rather than processing it.
The signature must be computed correctly or the test webhook secret must be explicitly configured. If the secret changes in the future, the test will fail silently at the webhook validation layer before reaching the user mismatch check.
🤖 Prompt for AI Agents
In `@tests/sentry/integrations/github/test_integration.py` around lines 398 - 454,
The test test_github_user_mismatch fails because the hardcoded
HTTP_X_HUB_SIGNATURE value does not match the HMAC-SHA1 of the webhook payload
(INSTALLATION_EVENT_EXAMPLE) given the default empty webhook secret, causing the
webhook to be rejected before the user-mismatch logic runs; to fix, either set
the webhook secret explicitly via options.set("github-app.webhook-secret",
"<secret>") in the test setup and use the matching signature header, or compute
the correct HMAC-SHA1 for INSTALLATION_EVENT_EXAMPLE with the secret used and
replace the HTTP_X_HUB_SIGNATURE header in the client.post call so the signature
validation in the GitHub webhook endpoint passes and the test exercises
GitHubIntegrationTest::test_github_user_mismatch logic.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
New Features
Improvements